Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change symbology #31

Merged
merged 6 commits into from
Feb 8, 2016
Merged

Conversation

Nemo157
Copy link
Contributor

@Nemo157 Nemo157 commented Feb 2, 2016

  • Change $ -> ^ and # -> @.
  • Add in support for render by move (with test showing it will still default to by-ref if needed).
  • Add in some more tests.
  • Remove symbol from #else (I guess this could break using an element called else, but who's gonna do that?)

Fixes #21, #24 and #26.

let r = R("pinkie ");
html!(s, ^r).unwrap();
html!(s, ^r).unwrap();
// R is not-Copyable so this shows that it will auto-ref splice arguments that implement Render.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's the test showing that the compiler auto-refs the spliced values.

@lambda-fairy
Copy link
Owner

👍 for using trait auto-ref.

I'll have to disagree on removing the sigil from else. Though I admit it looks cleaner, as with the shorthand class syntax it makes the grammar ambiguous, and precludes us from adding XML support in the future. That's not worth removing one character over IMO.

The rest of the patch looks good, so I'll be happy to merge once this is addressed.

@Nemo157
Copy link
Contributor Author

Nemo157 commented Feb 7, 2016

Ah, yep, I did wonder why it would be needed, but XML support makes sense. I'll add it back.

@lambda-fairy lambda-fairy merged commit 1b30744 into lambda-fairy:master Feb 8, 2016
lambda-fairy added a commit that referenced this pull request Feb 8, 2016
@Nemo157 Nemo157 deleted the change-symbology branch February 8, 2016 13:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants